Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update website after changing tag value #250

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

areebahmeddd
Copy link
Member

Signed-off-by: Areeb Ahmed [email protected]

Summary of Changes

Fixes: #122

  • Modified the product_tag_update function to return the updated tag value after an update.
  • Added test cases to verify that the updated tag value is returned correctly.

@areebahmeddd areebahmeddd requested a review from a team as a code owner March 8, 2025 22:46
@areebahmeddd
Copy link
Member Author

Needs testing. I'll update this message after testing

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.55%. Comparing base (b9cda65) to head (d9ed24c).
Report is 66 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #250      +/-   ##
==========================================
- Coverage   95.06%   94.55%   -0.52%     
==========================================
  Files           5        5              
  Lines         324      367      +43     
==========================================
+ Hits          308      347      +39     
- Misses         16       20       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@areebahmeddd areebahmeddd requested a review from aleene March 9, 2025 13:27
@alexgarel
Copy link
Member

@areebahmeddd it won't fix #122, because there is a part in openfoodfacts-server javascript, so please remove the "Fix:" in your description (which would close the issue). Or if you close the issue, please open a new issue on openfoodfacts-server to ask to leverage this new API feature in the javascript to display the property.

Comment on lines +472 to +473
# Return the updated tag instead of just "ok"
return product_tag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change in the API we must ensure no depending app will crash because of this…

One way to avoid this is to add a parameter to ask for the product_tag as return value, and return "ok" otherwise.

It might be that mobile app only use the response http status code, in which case it is ok, but please ask them first (opening an issue in their github repository).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants